Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated TracePoint #1943

Merged
merged 1 commit into from
Aug 9, 2024
Merged

Conversation

sampersand
Copy link
Contributor

@sampersand sampersand commented Aug 1, 2024

This PR updates TracePoint. There's a few idiosyncrasies (especially with some tests) that're discussed in the review comments

@sampersand sampersand force-pushed the swesterman/23-09-10/trace_point branch from c2caf8c to c986477 Compare August 1, 2024 05:40
core/trace_point.rbs Show resolved Hide resolved
core/trace_point.rbs Show resolved Hide resolved
@@ -257,7 +257,7 @@ class TracePoint < Object
# C.foo
# end
#
def defined_class: () -> Module
def defined_class: () -> (Class | Module)?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though Class | Module is technically Module, i think this is much clearer for the end-user

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Give the type a try.

core/trace_point.rbs Show resolved Hide resolved
test/stdlib/TracePoint_test.rb Show resolved Hide resolved
@@ -421,7 +421,7 @@ class TracePoint < Object
# Value from exception raised on the `:raise` event, or rescued on the `:rescue`
# event.
#
def raised_exception: () -> untyped
def raised_exception: () -> Exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the case of other events than :raise nor :rescue?

Suggested change
def raised_exception: () -> Exception
def raised_exception: () -> Exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you call raised_exception and there's no exception, it raises an error:

$ ruby -e 'TracePoint.trace(:line) { p _1.raised_exception }; p 1'
<internal:trace_point>:402:in `raised_exception': not supported by this event (RuntimeError)
	from -e:1:in `block in <main>'
	from -e:1:in `<main>'

@soutaro soutaro added this pull request to the merge queue Aug 9, 2024
@soutaro soutaro added this to the RBS 3.6 milestone Aug 9, 2024
Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪

Merged via the queue into ruby:master with commit e085076 Aug 9, 2024
19 checks passed
@soutaro soutaro added the Released PRs already included in the released version label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

2 participants